Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a "light" method protocol option to support specifying the method in the URL #5

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tyler-ham
Copy link
Contributor

With the "light" method protocol, clients can encode the method name in the URL like <controller>/<action> instead of calling <controller> and including the method in the JSON RPC request body.

One advantage of the light method format is that web server access logs will contain the method invoked because it is in the URL. The standard "heavy" format will not, because the method only appears inside the POST data.

The commit includes:

  • minor adjustments to Controller.php,
  • a new LightMethodProtocolTrait.php that a developer can import in their Controller to enable this alternate version of the protocol, and
  • additional usage documentation in the README

@cranetm
Copy link
Owner

cranetm commented Sep 25, 2015

there is a typo in example object under
"However, with the LightMethodProtocol, a client could instead post to"
and i think it is better to create ControllerLight instead of trait

@tyler-ham
Copy link
Contributor Author

I added a commit to fix the typos.

My first instinct of the addition was in fact a LightController subclass of Controller instead of a Trait. Here is why I thought it might be better as a Trait:

I have several additions in mind for the library: the Auth extension (already merged), this LightMethodProtocol option (in this pull request), a FormPostProtocol option (currently a work-in-progress), and I imagine we may come across other extensions (https://jsonrpcx.org/Main/HomePage) or options we may want to consider further down the road.

Implementing each extension/option as a subclass of Controller does make it easier for a developer to use since they only need to extend the right subclass of Controller, but it makes it harder to mix and match multiple options unless we provide a subclass of Controller for each combination of options.

However, by implementing the options as Traits, a developer could do the following:

class ServicesController extends \JsonRpc2\Controller
{
    use \JsonRpc2\LightMethodProtocolTrait;
    use \JsonRpc2\extensions\AuthTrait;
    use \JsonRpc2\FormPostProtocolTrait;
    use \JsonRpc2\FutureOptionalProtocolTrait;
    use \JsonRpc2\extensions\StatusTrait;
    use \JsonRpc2\extensions\ControlTrait;
}

and easily enable or disable individual options/extensions like:

class ServicesController extends \JsonRpc2\Controller
{
    use \JsonRpc2\LightMethodProtocolTrait;
    //use \JsonRpc2\extensions\AuthTrait;
    //use \JsonRpc2\FormPostProtocolTrait;
    use \JsonRpc2\SomeFutureOptionalProtocolTrait;
    //use \JsonRpc2\extensions\StatusTrait;
    use \JsonRpc2\extensions\ControlTrait;
}

As the number of options/extensions grows in the future, I believe this is a more elegant solution than creating a Controller subclass for each scenario.

It might be worth considering implementing both options to some extent. In addition to the traits, we could provide a couple of Controller subclasses for developer convenience, like

namespace JsonRpc2;
class ControllerExtended extends \JsonRpc2\Controller
{
    use \JsonRpc2\LightMethodProtocolTrait;
    use \JsonRpc2\extensions\AuthTrait;
    use \JsonRpc2\FormPostProtocolTrait;
    use \JsonRpc2\FutureOptionalProtocolTrait;
    use \JsonRpc2\extensions\StatusTrait;
    use \JsonRpc2\extensions\ControlTrait;
}

class ControllerLight extends \JsonRpc2\Controller
{
    use \JsonRpc2\LightMethodProtocolTrait;
}

This way, a developer could use one of the default Controller instances, but also have the option of using individual Traits to mix and match as needed for their particular application.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants